Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough온보딩 V2를 도입하며 대학/분야 조회 엔드포인트와 V2 온보딩 POST를 추가하고, 온보딩 DTO/응답, 상수(대학/분야), Member 엔티티 필드 및 서비스 onboarding 로직(비자 처리 포함)을 확장합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Ctrl as MemberControllerV2
participant Svc as MemberService
participant Mem as Member
participant Visa as MemberVisa
participant DB as Database
Client->>Ctrl: POST /api/v2/members/onboard (memberId, MemberOnboardV2Request)
Ctrl->>Svc: onboardMemberV2(request, memberId)
Svc->>DB: find member by id
DB-->>Svc: Member entity
Svc->>Mem: updateInfo(..., university, englishLevel, fieldsOfInterests, preparationStatuses, ...)
Mem->>DB: persist Member
Svc->>Visa: create MemberVisa(visaType, start, expired, visaPoint)
Visa->>DB: save MemberVisa
Svc-->>Ctrl: 성공
Ctrl-->>Client: 200 OK (BaseResponse<Void>)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/sopt/kareer/domain/member/entity/Member.java (1)
85-111:⚠️ Potential issue | 🔴 CriticalV2 온보딩에서 받은 신규 값들이 전부 유실됩니다.
src/main/java/org/sopt/kareer/domain/member/service/MemberService.java:98-105는 대학, 영어 레벨, 관심 분야, 준비 상태를 여기로 넘기고 있지만, 이 메서드는 그 값을 엔티티 필드에 한 번도 대입하지 않습니다. 지금 상태로는 온보딩 성공 응답 이후에도 네 값이 저장되지 않습니다.예시 수정
public void updateInfo(String name, LocalDate birthDate, Country country, String university, EnglishLevel englishLevel, String fieldsOfInterests, String preparationStatuses, LanguageLevel languageLevel, Degree degree, LocalDate expectedGraduationDate, String primaryMajor, String secondaryMajor, String targetJob, String targetJobSkill) { assertPendingStatus(); this.name = name; this.birthDate = birthDate; this.country = country; + this.university = university; + this.englishLevel = englishLevel; + this.fieldsOfInterest = fieldsOfInterests; + this.preparationStatus = preparationStatuses; this.languageLevel = languageLevel; this.degree = degree; this.expectedGraduationDate = expectedGraduationDate; this.primaryMajor = primaryMajor; this.secondaryMajor = secondaryMajor;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/kareer/domain/member/entity/Member.java` around lines 85 - 111, The updateInfo method in Member (updateInfo) never assigns the incoming university, englishLevel, fieldsOfInterests, and preparationStatuses parameters to the entity, so values passed from MemberService are lost; update the method to set this.university = university, this.englishLevel = englishLevel, this.fieldsOfInterests = fieldsOfInterests, and this.preparationStatuses = preparationStatuses (keeping the existing assignments for other fields and status and preserving assertPendingStatus() behavior).
🧹 Nitpick comments (2)
src/main/java/org/sopt/kareer/domain/member/dto/request/MemberOnboardV2Request.java (2)
34-35:expectedGraduationDate에 대한 cross-field 검증 누락.
visaPoint는isVisaPointValid()메서드로 D10 비자일 때만 필수/허용되도록 검증하고 있습니다. 그러나expectedGraduationDate는 Schema 설명에 "D2 비자인 경우만"이라고 명시되어 있음에도 불구하고 동일한 cross-field 검증이 없습니다. D2가 아닌 경우에도 값이 입력될 수 있고, D2인 경우 필수 여부도 검증되지 않습니다.비즈니스 요구사항에 따라
isExpectedGraduationDateValid()메서드를 추가하는 것을 고려해 주세요.♻️ 예시 구현
`@AssertTrue`(message = "예상 졸업일은 D2 비자인 경우에만 입력할 수 있습니다.") `@Schema`(hidden = true) public boolean isExpectedGraduationDateValid() { if (visaType == null) { return expectedGraduationDate == null; } if (visaType == VisaType.D2) { return expectedGraduationDate != null; } return expectedGraduationDate == null; }Also applies to: 65-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/kareer/domain/member/dto/request/MemberOnboardV2Request.java` around lines 34 - 35, MemberOnboardV2Request is missing cross-field validation for expectedGraduationDate (field expectedGraduationDate) against visaType; add a boolean validator method isExpectedGraduationDateValid() (annotated with `@AssertTrue` and `@Schema`(hidden = true)) that enforces: when visaType == null then expectedGraduationDate must be null, when visaType == VisaType.D2 then expectedGraduationDate must be non-null, and for any other visaType expectedGraduationDate must be null; place this method in the MemberOnboardV2Request class alongside isVisaPointValid() to satisfy the Schema requirement "D2 비자인 경우만."
16-17:university필드에@NotNull대신@NotBlank사용 권장.다른 String 필드들(
name,primaryMajor,targetJob)은@NotBlank를 사용하여 빈 문자열을 방지하고 있습니다.university도 String 타입이므로 일관성과 빈 문자열 방지를 위해@NotBlank를 사용하는 것이 적절합니다.♻️ 수정 제안
- `@NotNull`(message = "대학교는 필수 입력값입니다.") + `@NotBlank`(message = "대학교는 필수 입력값입니다.") String university,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/kareer/domain/member/dto/request/MemberOnboardV2Request.java` around lines 16 - 17, Replace the `@NotNull` on the university field in MemberOnboardV2Request with `@NotBlank` to prevent empty strings and keep validation consistent with other String fields (name, primaryMajor, targetJob); update the import to javax.validation.constraints.NotBlank if needed and ensure the validation message remains appropriate for the university field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/kareer/domain/member/controller/MemberControllerV2.java`:
- Around line 18-29: The OnboardingRestrictionFilter currently allows only the
v1 onboarding path, preventing PENDING users from reaching
MemberControllerV2.onboardMember; update the filter (class
OnboardingRestrictionFilter) to include the v2 onboarding pattern (e.g.
"/api/v2/members/onboard" or "/api/v2/members/onboard/**") in the allowed/exempt
paths collection or matcher list so that requests to MemberControllerV2's POST
/onboard are bypassed by the onboarding restriction check.
In
`@src/main/java/org/sopt/kareer/domain/member/dto/request/MemberOnboardV2Request.java`:
- Around line 51-52: The fieldsOfInterests declaration in MemberOnboardV2Request
currently uses only `@NotNull` so an empty list or >5 entries still pass
validation despite the message; add a collection size constraint (e.g.,
`@Size`(min = 1, max = 5)) to the fieldsOfInterests field so the validation
enforces "최소 1개, 최대 5개" alongside `@NotNull`, importing the appropriate
javax.validation.constraints.Size if needed.
In
`@src/main/java/org/sopt/kareer/domain/member/entity/constants/University.java`:
- Line 111: The University enum contains malformed/concatenated names (e.g.,
"Chongjunationaluniversityofeducation", "Busandigital University",
"Daejeoncatholic University") that will surface directly to users; open the
University enum and replace those constants with their correctly spaced/cased
names ("Chongju National University of Education", "Busan Digital University",
"Daejeon Catholic University") and then add a simple normalization/validation
unit test (or a static initializer in University) to assert every entry contains
at least one space and uses proper word boundaries so future malformed inputs
fail CI.
---
Outside diff comments:
In `@src/main/java/org/sopt/kareer/domain/member/entity/Member.java`:
- Around line 85-111: The updateInfo method in Member (updateInfo) never assigns
the incoming university, englishLevel, fieldsOfInterests, and
preparationStatuses parameters to the entity, so values passed from
MemberService are lost; update the method to set this.university = university,
this.englishLevel = englishLevel, this.fieldsOfInterests = fieldsOfInterests,
and this.preparationStatuses = preparationStatuses (keeping the existing
assignments for other fields and status and preserving assertPendingStatus()
behavior).
---
Nitpick comments:
In
`@src/main/java/org/sopt/kareer/domain/member/dto/request/MemberOnboardV2Request.java`:
- Around line 34-35: MemberOnboardV2Request is missing cross-field validation
for expectedGraduationDate (field expectedGraduationDate) against visaType; add
a boolean validator method isExpectedGraduationDateValid() (annotated with
`@AssertTrue` and `@Schema`(hidden = true)) that enforces: when visaType == null
then expectedGraduationDate must be null, when visaType == VisaType.D2 then
expectedGraduationDate must be non-null, and for any other visaType
expectedGraduationDate must be null; place this method in the
MemberOnboardV2Request class alongside isVisaPointValid() to satisfy the Schema
requirement "D2 비자인 경우만."
- Around line 16-17: Replace the `@NotNull` on the university field in
MemberOnboardV2Request with `@NotBlank` to prevent empty strings and keep
validation consistent with other String fields (name, primaryMajor, targetJob);
update the import to javax.validation.constraints.NotBlank if needed and ensure
the validation message remains appropriate for the university field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d2ccc091-8337-486f-9e4c-136f54519608
📒 Files selected for processing (9)
src/main/java/org/sopt/kareer/domain/member/controller/MemberController.javasrc/main/java/org/sopt/kareer/domain/member/controller/MemberControllerV2.javasrc/main/java/org/sopt/kareer/domain/member/dto/request/MemberOnboardV2Request.javasrc/main/java/org/sopt/kareer/domain/member/dto/response/OnboardFieldsResponse.javasrc/main/java/org/sopt/kareer/domain/member/dto/response/OnboardUniversitiesResponse.javasrc/main/java/org/sopt/kareer/domain/member/entity/Member.javasrc/main/java/org/sopt/kareer/domain/member/entity/constants/Field.javasrc/main/java/org/sopt/kareer/domain/member/entity/constants/University.javasrc/main/java/org/sopt/kareer/domain/member/service/MemberService.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue 🛠
Work Description 📝
ScreenShots 📷
To Reviewers 📢
Summary by CodeRabbit
새로운 기능
관리/운영